Skip to content

Migrate to xunit v3#7273

Draft
zivkan wants to merge 4 commits intodevfrom
dev-zivkan-xunit3
Draft

Migrate to xunit v3#7273
zivkan wants to merge 4 commits intodevfrom
dev-zivkan-xunit3

Conversation

@zivkan
Copy link
Copy Markdown
Member

@zivkan zivkan commented Apr 13, 2026

Bug

Fixes: updating infrastructure

Description

Upgrade xunit to v3. It has some breaking changes, so the following changes were made:

  • The namespace of ITestOutputHelper changed, so every file that was using it needed to remove the old namespace, and if the xunit namespace wasn't already used, add a using for that.
  • The PM UI tests using [WPFFact] and [WPFTheory] can use xunit's, rather than needing our own definition of them.
  • xunit no longer allows async void. There was one instance that needed to be changed to async Task.
  • The way xunit attributes work has changed, so all the custom attributes in Test.Utilities, like [PlatformFact] are changed. NuGet.CommandLine.Tests also has one.
  • Test Resolve_WhenPackageExists_ReturnsSucceededSdkResult in Microsoft.Build.NuGetSdkResolver.Tests's NuGetSdkResolver_Tests.cs was validating that the output contains the message about X.509 cert chain. However, that message is output when a static variable is set to false, meaning the first method to run the test will output the message and set the static to true, preventing it from outputting for any other test. The validation only worked by coincidence because xunit runs tests in a deterministic order and this happened to be the first test in the assembly that calls into the static method that checks and changes the static variable. xunit v3 changes the test run order, so the test is no longer first, and therefore no longer gets output. Given the name of the test isn't related to checking this output message, I deleted the validation
    • An alternative to deleting the validation is to use a class fixture that resets the static variable before every test run, in the class. This would still have a risk that tests from other classes running in parallel might call the static method and therefore make the test validation the output flakey. If someone feels strongly about validating this message, let me know. A Dotnet.Integration.Test would probably be better off, since that wouldn't introduce timing risks.
  • The server team uses our Microsoft.Internal.NuGet.Testing.SignedPackages package, but they don't use the ChildProcess helper method. So, removing xunit should be a non-breaking change, although it depends if they depend on the specific exception type that was being thrown. The reason I removed the xunit references is because 1. ChildProcess and RetryRunner using IOutputTestHelper from a different package/assembly is a breaking change and 2. I don't know how xunit v3's analyzers are implemented, and I wanted to reduce risk of the xunit v3 analyzers causing build errors in the server team's repo if they're not using xunit v3.

PR Checklist

  • Meaningful title, helpful description and a linked NuGet/Home issue
  • Added tests N/A
  • Link to an issue or pull request to update docs if this PR changes settings, environment variables, new feature, etc.

@zivkan zivkan force-pushed the dev-zivkan-xunit3 branch from 9fa5511 to bc6f78e Compare April 13, 2026 08:05
@zivkan zivkan force-pushed the dev-zivkan-xunit3 branch from bc6f78e to 7bda1ab Compare April 13, 2026 08:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant